feat: abstract around base package manager #22
Conversation
|
I think we're on the right path here with enabling support for Slurm in both snap and deb form, but I'm wondering where we can "duplicate" things between package formats such that we don't need to have as much abstraction happening in the background to manipulate things like the munge key file and Slurm configuration in the background. Re. the Slurm configuration, we can just manipulate that with |
|
@NucciTheBoss Yeah, using mungectl and slurmutils on both snap and deb mode would be nice. In the meantime, I think we can just push the current abstractions and see if we can unify things later. |
|
Right, this won't pass CI until we fix charmed-hpc/slurm-snap#47 |
Yeah... I think it's time for |
|
@jedel1043 is this PR ready for another review, or are you still working on implementation the configuration management? We could follow on with that in a separate PR since we still need to implement the deb manager, and that PR will give us a better idea of how that implementation needs to shape up. |
|
@NucciTheBoss We need to integrate |
Bout ready to go YOLO and nuke |
|
Wow... CI passed... |
jedel1043
force-pushed
the
slurm-deb-enablement
branch
2 times, most recently
from
7efe2b0 to
f1234df
Compare
There was a problem hiding this comment.
I'm quite happy with how this has shaped up. Just a couple suggested changes and I'm ready to merge this so we can get focused on Terraform x AMD GPU stuff 🤩
- I don't think we're using the
format_keyfunction anymore since we're editing the config files directly. - I think we can worry about linting the passed environment variables later when we have a better idea of where inputs are coming from within the Slurm charms. I think right now we just set/get/unset with no frills about it.
- Should we define the Slurm service managers in their own charms respectively? I'm thinking about that discussion we had a while ago where we didn't want to put the managers into this lib because then we'd need to update the lib everywhere everytime we made a change to the manager.
|
This is looking great @jedel1043 🎉 I just replied to your comment about the kebab casing for |
This should enable switching between snaps and debs in any charm using the library. It also exposes the slurmutils config models instead of having wrapper methods around it, which overall moves the burden of config APIs to slurmutils.
jedel1043
force-pushed
the
slurm-deb-enablement
branch
from
3e1e393 to
d606610
Compare
|
On second thought, it would probably be better to implement the deb manager before bumping version. That way the lib is "complete" before building it into the Slurm charms. |
This PR abstracts around the base package manager used.
The objective of this is to enable switching between snap packages and deb packages in a more composable way.